To get started, run npm run start-debug and open http://0.0.0.0:9966/debug/shield-rotation.html#10/39.8087/-86.0879 in your browser. You should see something like this:

It is good that the shields are grouped, but their orientation should be vertical.

The code is copy-pasted from https://github.com/ZeLonewolf/maplibre-shield-rotation-sample and was written by @ZeLonewolf

Bundle size report:

Size Change: +42 B
Total Size Before: 194 kB
Total Size After: 194 kB

If text-rotate is set, glyph quads will be rotated by the following code:

The glyph quads will rotate more later to align with the orientation of the line.

I think I found a way to achieve what we want:

For this what I changed was this:

Which I hard-coded to zero, highp float segment_angle = 0.0.

Of course, this is not sustainable as it basically breaks the rest of the library but at least I found a line in the code-base where a tweak will stop symbols and text from being rotated.

That preview looks awesome! It would have taken me forever just to find the spot in the codebase where that code even happens.

Doing this in the shaders as suggested above is probably the worst option. We can achieve the same result by hard-coding something in a TypeScript function called placeGlyphAlongLine(), which seems already a bit better:

If I make the return statement this:

    return {
        point: p,
        angle: 0.0, // segmentAngle,
        path: pathVertices
    };

I also get upright shields.

This hack does not work with rotated or pitched maps. What would be your expectation if the map is rotated or pitched?

For a rotated or tilted map, I would expect the shields to tilt while maintaining a vertical orientation. What does ordinary text currently do on rotation? Does it maintain an upright bias like it does on an unrotated/untilted map?

'text-pitch-alignment': 'viewport' and the hard-coded zero angle in placeGlyphAlongLine() gives the following result:

This is probably quite close to what we want. It is just a bit strange that at zero pitch, i.e., default view which is perpendicular to the map, the shield looks quite bad (worse than without the text-pitch-alignment).

Without text-pitch-alignement (good):

With text-pitch-alignment without pitching the map (bad):

With text-pitch-alignment with pitching the map (good):

I added a new boolean style property called text-rotate-to-line. If true (default), the text rotates to the orientation of the line. If you set it to false, letters will not care about the rotation of the line they are attached to. This is the case we want to have for the highway shields.

@ZeLonewolf can you try it out and give me some feedback?

Thanks @wipfli I would be thrilled to test this out. I'll let you know how it goes.

text-writing-mode was introduced for solving possibly similar issues with CJK characters, see mapbox/mapbox-gl-js#8399. My understanding so far is that it is limited to 'symbol-placement': 'point'.

Wow! This looks awesome in 2D in Indiana:

Pan and tilt:

We might want to change the orientation on the pan and tilt version, but this is already better than the status quo.

One thing I've noticed is that the corners of icons overlap when they're placed along a diagonal, but there's space between them when they're arranged vertically or horizontally. In the screenshots below, there is a single space character between each icon pair:

I wonder if MapLibre handles this correctly for collisions that have nothing to do with my change. Can you check what happens if you set text-rotate to 45 degrees, do the corners overlap on a horizontal line? I noticed that collision logic is handled in a separate file from the placement and rendering code. Maybe I forgot something there...

By the way I love all these little shields with their strange shapes. Some of them look like a map on the map.

With text-rotate: 45, on a vertical stack I do see overlap:

On a 45-degree road, I get the expected spacing:

@wipfli can you merge from main? I'm attempting to build this branch using npm, which is currently broken and fixed by #704.

If you want to build the library from source code and produce the files in the dist folder yourself, you can run:

nvm use 14
npm ci
npm run build-prod
ls dist
# maplibre-gl.css  maplibre-gl-dev.js  maplibre-gl.d.ts  maplibre-gl-unminified.js  maplibre-gl-unminified.js.map  package.json

Since text-rotate: 45 also results in overlapping shields, I think that my new feature behaves the same wrong way as the existing code does when it comes to rotated shields.

Since text-rotate: 45 also results in overlapping shields, I think that my new feature behaves the same wrong way as the existing code does when it comes to rotated shields.

I agree. This is an issue for letters also, which overlap when following lines with too tight of a radius.

Hey - quick question here - for onthegomap.com, I implemented highway shields by using:

  'layout': {
    'icon-image': [
      'concat',
      ['get', 'network'],
      '-',
      ['to-string', ['max', 2, ['get', 'ref_length']]],
    ],
    'icon-rotation-alignment': 'viewport',
    'symbol-placement': 'line',
    'symbol-spacing': 500,
    'text-field': ['to-string', ['get', 'ref']],
    'text-font': ['Robonoto Regular'],
    'text-offset': [0, 0.1],
    'text-rotation-alignment': 'viewport',
    'text-size': 10,
    'icon-size': 0.9,
  },

2 questions:

1. Would 'text-rotation-alignment': 'viewport' and 'icon-rotation-alignment': 'viewport' do what you're trying to achieve @ZeLonewolf?
2. What is the benefit of dynamically constructing the image with text over using the approach of a text symbol and icon symbol like I'm using?

Also, I like the highway shield images! Are they available anywhere for other sites to use?

This is my MO basically:

1. Start writing a unit test.
2. Mock the hell out of everything until I get to the relevant part of the code which I'm interested in testing.
3. Copy stuff from other tests to pass at part of the step above.
4. Make the test pass.

Here's my initial code, far from complete at it fails somewhere in the middle of the code.
Unfortunately, I don't have more time tonight to continue working on it.
draw_symbol.test.ts:

import {SymbolLayerSpecification} from '../style-spec/types.g';
import EvaluationParameters from '../style/evaluation_parameters';
import SymbolStyleLayer from '../style/style_layer/symbol_style_layer';
import ZoomHistory from '../style/zoom_history';
import drawSymbol from './draw_symbol';
import Painter from './painter';
jest.mock('./painter');

describe('drawSymbol', () => {
    test('should not do anything', () => {
        const mockPainter = new Painter(null, null);
        mockPainter.renderPass = 'opaque';

        drawSymbol(mockPainter, null, null, null, null);

        expect(mockPainter.colorModeForRenderPass).not.toHaveBeenCalled();
    });

    test('should draw and rotate glyphs according to the line', () => {
        const mockPainter = new Painter(null, null);
        mockPainter.context = {
            gl: jest.fn() as any
        } as any;
        mockPainter.renderPass = 'translucent';
        const layerSpec = {
            id: 'mock-layer',
            source: 'empty-source',
            type: 'symbol',
            layout: {
                'text-variable-anchor': ['center']
            },
            paint: {}
        } as SymbolLayerSpecification;
        const layer = new SymbolStyleLayer(layerSpec);
        layer.recalculate({zoom: 0, zoomHistory: {} as ZoomHistory} as EvaluationParameters, []);
        // a lot more mocking is needed here, I copy it from different tests until it passes the line of code I'm not interested in, in the production code
        drawSymbol(mockPainter, null, layer, [], null);

        // expect program.draw to be called with specific parameters
    });
});

The following is the entire test file that gets to the program.draw method:

import { mat4 } from 'gl-matrix';
import SymbolBucket from '../data/bucket/symbol_bucket';
import SourceCache from '../source/source_cache';
import Tile from '../source/tile';
import {OverscaledTileID} from '../source/tile_id';
import {SymbolLayerSpecification} from '../style-spec/types.g';
import EvaluationParameters from '../style/evaluation_parameters';
import SymbolStyleLayer from '../style/style_layer/symbol_style_layer';
import ZoomHistory from '../style/zoom_history';
import drawSymbol from './draw_symbol';
import Painter from './painter';
import Program from './program';
jest.mock('./painter');
jest.mock('./program');
jest.mock('../source/source_cache');
jest.mock('../source/tile');
jest.mock('../data/bucket/symbol_bucket');

describe('drawSymbol', () => {
    test('should not do anything', () => {
        const mockPainter = new Painter(null, null);
        mockPainter.renderPass = 'opaque';

        drawSymbol(mockPainter, null, null, null, null);

        expect(mockPainter.colorModeForRenderPass).not.toHaveBeenCalled();
    });

    test('should update variable anchors', () => {

        const mockPainter = new Painter(null, null);
        mockPainter.context = {
            gl: {},
            activeTexture: {
                set: () => {}
            }
        } as any;
        mockPainter.renderPass = 'translucent';
        mockPainter.transform = { pitch: 0, labelPlaneMatrix: mat4.create() } as any;
        mockPainter.options = {} as any;

        const layerSpec = {
            id: 'mock-layer',
            source: 'empty-source',
            type: 'symbol',
            layout: {
                //'text-variable-anchor': ['center']
            },
            paint: {
                'text-opacity': 1
            }
        } as SymbolLayerSpecification;
        const layer = new SymbolStyleLayer(layerSpec);
        layer.recalculate({zoom: 0, zoomHistory: {} as ZoomHistory} as EvaluationParameters, []);
        
        const tileId = new OverscaledTileID(1, 0, 1, 0, 0);
        tileId.posMatrix = mat4.create();
        const programMock = new Program(null, null, null, null, null, null);
        (mockPainter.useProgram as jest.Mock).mockReturnValue(programMock);
        const bucket = new SymbolBucket(null);
        bucket.icon = {
            programConfigurations: {
                get: () => {}
            },
            segments: {
                get: () => [1]
            }
        } as any;
        bucket.iconSizeData = {
            kind: 'constant',
            layoutSize: 1
        }
        const tile = new Tile(tileId, 256);
        tile.tileID = tileId;
        tile.imageAtlasTexture = {
            bind: () => {}
        } as any;
        (tile.getBucket as jest.Mock).mockReturnValue(bucket);
        const sourceCacheMock = new SourceCache(null, null, null);
        (sourceCacheMock.getTile as jest.Mock).mockReturnValue(tile);
        sourceCacheMock.map = { showCollisionBoxes: false } as any;

        drawSymbol(mockPainter, sourceCacheMock, layer, [tileId], null);

        expect(programMock.draw).toHaveBeenCalledTimes(1);
    });
});

I have added a lot of typings to the draw_symbol.ts in order to better understand how it works.
I'll create a PR for this as it is not related to this issue but a general improvement.
I need to see if it collides with the terrain 3d and fix the relevant conflicts.
PR will be sent shortly.

This should conflict, shouldn't it?

Merged main into shield-rotation without conflicts. Where did you expect a collision?

In the draw symbol file. But I might have underestimated the merge engine :-)

Are you now able to write the relevant test(s) or is my assistance still needed?

Thanks for adding that test @HarelM. Sorry if I am a bit slow to understand here, but which part of draw_symbol.ts should we cover with unit testing? Currently not coverd in draw_symbol.ts are the lines

70,114-235,240,242,306-313,334,346-351,374-377,393,402-404,409-414

I didn't cover everything in that PR. I suggest only to add a test for this logic that was changed as part of this PR.

In general, we should cover as much as possible. But I don't want to hold this PR due to low coverage.

Overall very nice!
There's a need to take into consideration the auto possibility and make sure it is working as expected with all the ifs.
Other than that, it looks good!

I think this PR should wait until #1022 will be merges since I think solving conflicts in this PR will be easier...

Thanks for the auto hint. I added a render test to text-pitch-alignment: auto since this is what will be affected. I am fine with waiting until terrain3d is merged!

Cool, I'll approve, you can decide if you want to merge this into main and then merge to terrain3d or wait, your call.

Thanks for reviewing @HarelM. I will merge this to main and then merge main into #1022.